Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Custom user callbacks (custom requests) #14

Merged
merged 6 commits into from
Apr 4, 2024

Conversation

bigbrett
Copy link
Contributor

@bigbrett bigbrett commented Apr 2, 2024

First stab at simple user callback implementation. It allows for multiple (up to 255) "custom request callbacks" to be registered by the server, and allows for these callbacks to be requested by the client via the client API. It supports passing arbitrary data in and out of the server callback. It also provides a query functionality to check if a given callback has been registered.

@bigbrett bigbrett requested review from billphipps and jpbland1 April 2, 2024 20:46
@bigbrett bigbrett force-pushed the custom-message-handling branch from 903e5cb to 12c810e Compare April 2, 2024 21:17
Copy link
Contributor

@billphipps billphipps left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Gotta move the state into server context and please make it smaller.

src/wh_server_custom.c Outdated Show resolved Hide resolved
wolfhsm/wh_server_custom.h Outdated Show resolved Hide resolved
wolfhsm/wh_message_custom.h Outdated Show resolved Hide resolved
wolfhsm/wh_message_custom.h Outdated Show resolved Hide resolved
test/wh_test_clientserver.c Outdated Show resolved Hide resolved
src/wh_message_custom.c Outdated Show resolved Hide resolved
src/wh_client.c Outdated Show resolved Hide resolved
- made custom callback table part of server context
- reduced number of callbacks to 8
- deleted wh_server_custom.h and moved contents to wh_server.h
- Added wh_Client_CustomResponseCheckRegistered() helper
billphipps
billphipps previously approved these changes Apr 4, 2024
Copy link
Contributor

@billphipps billphipps left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great! Minor nits, but I'm good with it at this point.

@@ -12,6 +12,7 @@
#include "wolfhsm/wh_common.h"
#include "wolfhsm/wh_comm.h"
#include "wolfhsm/wh_nvm.h"
#include "wolfhsm/wh_message_custom.h"
Copy link
Contributor

@billphipps billphipps Apr 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't see where this was needed in this header.

- Renamed custom structs and functions to customcb
- renamed files
- added blocking client helper
- changed request/response id from uint16_t to uint32_t
@bigbrett bigbrett force-pushed the custom-message-handling branch from b1a94bb to 1029307 Compare April 4, 2024 02:10
Copy link
Contributor

@billphipps billphipps left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! I'm going to push this in now. Here's a short list of minor tweaks:

  1. Rename to WH_ERROR_NOHANDLER
  2. Remove WH_ACTION_MAX if not needed
  3. Add 1 to actions so we can keep action ==0 as invalid.
  4. Add prototype for busy wait CheckRegistered to header.

@billphipps billphipps merged commit b92437a into wolfSSL:main Apr 4, 2024
1 check passed
@bigbrett bigbrett mentioned this pull request Apr 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants